Ef 18281 brackets too wide - #77
Open
Ikreb1 wants to merge 6 commits into
Open
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Pull request overview
This PR updates Trinity’s bracket projection logic to use oriented bounding boxes (OBBs) instead of axis-aligned bounding boxes (AABBs), aiming to produce tighter on-screen bracket rectangles for rotated/scaled objects. It also adds an optional debug visualization path to render the consumed bounds and the published screen rect.
Changes:
- Replace AABB-based projection in
Tr2ProjectBoundingBoxBracketwith OBB corner projection (including near-plane intersection handling). - Update the
ITr2BoundingBoxinterface and key implementers to provideGetWorldBoundingObb(Obb&). - Add a
debugDrawattribute to visualize the OBB wireframe and projected rect via the debug renderer.
File summaries
| File | Description |
|---|---|
| trinity/Tr2ProjectBoundingBoxBracket.h | Adds m_debugDraw flag for optional debug visualization. |
| trinity/Tr2ProjectBoundingBoxBracket.cpp | Switches projection to OBB-based math; adds debug draw helpers for OBB + rect. |
| trinity/Tr2ProjectBoundingBoxBracket_Blue.cpp | Exposes debugDraw to Blue so it can be toggled at runtime. |
| trinity/Interior/Tr2IntSkinnedObject.h | Adds GetWorldBoundingObb declaration for the bounding interface. |
| trinity/Interior/Tr2IntSkinnedObject.cpp | Implements GetWorldBoundingObb via Obb::CreateClippedWorldBoundingObb. |
| trinity/Interior/Tr2InteriorPlaceable.h | Adds GetWorldBoundingObb declaration for placeables. |
| trinity/Interior/Tr2InteriorPlaceable.cpp | Implements GetWorldBoundingObb for placeables. |
| trinity/Include/ITr2BoundingBox.h | Changes interface from GetWorldBoundingBox(min,max) to GetWorldBoundingObb(obb). |
| trinity/Eve/SpaceObject/EveSpaceObject2.h | Updates interface method to GetWorldBoundingObb. |
| trinity/Eve/SpaceObject/EveSpaceObject2.cpp | Implements GetWorldBoundingObb using local AABB + world transform. |
| trinity/Eve/EveTransform.h | Updates interface method to GetWorldBoundingObb. |
| trinity/Eve/EveTransform.cpp | Implements GetWorldBoundingObb using computed local bounds + world transform. |
| trinity/Eve/EvePlanet.h | Updates interface method to GetWorldBoundingObb. |
| trinity/Eve/EvePlanet.cpp | Implements GetWorldBoundingObb for a spherical planet as an OBB representation. |
| trinity/Eve/EveEffectRoot2.h | Updates interface method to GetWorldBoundingObb. |
| trinity/Eve/EveEffectRoot2.cpp | Implements GetWorldBoundingObb from bounding-sphere data (conservative under non-uniform scale). |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ikreb1
marked this pull request as ready for review
August 31, 2026 13:02
Ikreb1
force-pushed
the
EF-18281-brackets-too-wide
branch
from
August 31, 2026 13:02
a394cdc to
af9ea1f
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cpp-linter Review
Used clang-format v21.1.8
Click here for the full clang-format patch
diff --git a/trinity/Tr2ProjectBoundingBoxBracket.cpp b/trinity/Tr2ProjectBoundingBoxBracket.cpp
index aa58d41..ba2daed 100644
--- a/trinity/Tr2ProjectBoundingBoxBracket.cpp
+++ b/trinity/Tr2ProjectBoundingBoxBracket.cpp
@@ -24,3 +24,12 @@ const int OBB_EDGES[12][2] = {
- { 0, 1 }, { 2, 3 }, { 4, 5 }, { 6, 7 },
- { 0, 2 }, { 1, 3 }, { 4, 6 }, { 5, 7 },
- { 0, 4 }, { 1, 5 }, { 2, 6 }, { 3, 7 },
+ { 0, 1 },
+ { 2, 3 },
+ { 4, 5 },
+ { 6, 7 },
+ { 0, 2 },
+ { 1, 3 },
+ { 4, 6 },
+ { 5, 7 },
+ { 0, 4 },
+ { 1, 5 },
+ { 2, 6 },
+ { 3, 7 },
Have any feedback or feature suggestions? Share it here.
…box sides properly
…g in nullptr isnt needed anymore
…ed a function to create the obb from sphere
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Ikreb1
force-pushed
the
EF-18281-brackets-too-wide
branch
from
August 31, 2026 16:09
f33021f to
e6a8c88
Compare
CCPNobody
reviewed
Sep 3, 2026
| // Both endpoints must be on opposite sides of the near plane; the caller | ||
| // guarantees this via the outcode test. | ||
| void AddNearPlaneIntersection( const Vector4& a, const Vector4& b, std::vector<Vector4>& points ) | ||
| void AddNearPlaneIntersection( const Vector4& a, const Vector4& b, Vector4* points, int& pointCount ) |
Member
There was a problem hiding this comment.
what was the reason for changing the interface to this function?
Member
Author
There was a problem hiding this comment.
Old version was allocating on the heap every frame, I felt it was quite safe since no external consumer of this function and the size is known and asserted elsewhere
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://fenriscreations.atlassian.net/browse/EF-18281
change implementation of boundingbox calculation to use Obb instead of AABB which was not fitting to the boundingbox of the object close enough
Cyan: current
Magenta: improved with AABB->obb and corrected per-corner clip w (workaround for a broken carbon-math Transform overload)